Skip to content

Symbolicate JIT frames in Android native crash handler - #231

Merged
winnerspiros merged 2 commits into
masterfrom
copilot/fix-apk-crash-on-start-another-one
Apr 22, 2026
Merged

Symbolicate JIT frames in Android native crash handler#231
winnerspiros merged 2 commits into
masterfrom
copilot/fix-apk-crash-on-start-another-one

Conversation

Copilot AI commented Apr 22, 2026

Copy link
Copy Markdown

The uploaded build_test/native_crash.log shows a real SIGSEGV pc=0 (null function-pointer call) on SDLThread, but all 32 backtrace frames sit in Mono JIT/trampoline rwxp mappings where dladdr cannot resolve symbols — every frame prints <unresolved>, so the crash cannot be diagnosed.

Changes

  • osu.Android/Native/crash_handler.cpp — two new fallbacks tried in order when dladdr fails:
    • resolveViaPerfmap() lazily mmaps a Mono --jitmap file (searched in <dirname(g_logPath)>, $TMPDIR, /tmp, /data/local/tmp) and writes [JIT] <managed_method>+0xOFF.
    • resolveViaProcMaps() scans /proc/self/maps and writes [perms start-end +offset] <path>, tagging anonymous executable mappings as [Mono JIT/trampoline (anon rwxp)]. Always works, no setup required.
    • Both are wired into writeFrame() and unwindCallback(). All new code is async-signal-safe (open/read/mmap/write only; fixed stack buffers; no malloc/stdio).
  • osu.Android/mono.env + <AndroidEnvironment> reference in osu.Android.csproj — sets MONO_ENV_OPTIONS=--jitmap and redirects TMPDIR to the app's external-files dir so the perfmap lands next to native_crash.log (reachable via Files app on unrooted devices).

Result

A frame that previously printed:

#17 pc 0x000000753a10cc08  <unresolved>

will now print one of:

#17 pc 0x000000753a10cc08  [JIT] osu.Framework.Graphics.Drawable:UpdateSubTree+0x88
#17 pc 0x000000753a10cc08  [rwxp 0x753a0ff000-0x753a13f000 +0xdc08] [Mono JIT/trampoline (anon rwxp)]

depending on whether the perfmap is present.

Notes

  • A host-compiled unit test of the perfmap and /proc/self/maps line parsers caught and corrected an off-by-one in the maps field-skip count (was eating the path).
  • build_test/ left untouched per instruction.

Copilot AI and others added 2 commits April 22, 2026 07:30
…ash handler

Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/2246a408-aec9-460c-9b2b-ecb666b6fb37

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/2246a408-aec9-460c-9b2b-ecb666b6fb37

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
@winnerspiros
winnerspiros marked this pull request as ready for review April 22, 2026 07:35
Copilot AI review requested due to automatic review settings April 22, 2026 07:35
@winnerspiros
winnerspiros merged commit d67e2da into master Apr 22, 2026
4 of 18 checks passed
@gitar-bot

gitar-bot Bot commented Apr 22, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Android native crash diagnostics by adding fallback symbolication for Mono JIT/trampoline frames when dladdr() can’t resolve symbols, aiming to make native_crash.log actionable for managed-code and trampoline-related crashes.

Changes:

  • Add perfmap (MONO_ENV_OPTIONS=--jitmap) and /proc/self/maps-based fallback symbolication paths in crash_handler.cpp when dladdr() fails.
  • Add mono.env and wire it into the Android project via AndroidEnvironment to enable --jitmap and redirect TMPDIR for perfmap placement.
  • Adjust frame writing/unwind output to use the new fallback resolvers before printing <unresolved>.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
osu.Android/osu.Android.csproj Includes mono.env via AndroidEnvironment so Mono env vars are packaged into the APK.
osu.Android/mono.env Sets MONO_ENV_OPTIONS=--jitmap and redirects TMPDIR to place Mono perfmaps alongside crash logs.
osu.Android/Native/crash_handler.cpp Implements perfmap and /proc/self/maps fallback symbolication and integrates it into backtrace output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +312 to +317
size_t n = g_perfmapSize;
size_t i = 0;
while (i < n) {
// Each line: "<hex_start> <hex_size> <name>\n"
size_t lineStart = i;
size_t p = i;

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveViaPerfmap() does a full linear scan of the (up to) 64MB perfmap for every unresolved frame. In the worst case (many JIT frames) this can turn crash handling into a multi-hundred-ms / multi-second operation, increasing the chance the process is killed before the log is fully written. Consider reducing repeated work per crash (e.g. stop early if the perfmap is sorted by address, or add a small fixed-size cache of the last N lookups).

Copilot uses AI. Check for mistakes.
Comment on lines +439 to +451
outBuf[bp < (int)sizeof(outBuf) - 1 ? bp++ : bp] = ']';
if (path[0] != '\0') {
appendLit(outBuf, sizeof(outBuf), &bp, " ");
for (int k = 0; path[k] && bp < (int)sizeof(outBuf) - 1; ++k)
outBuf[bp++] = path[k];
} else if (perms[2] == 'x') {
// Anonymous executable mapping: classic Mono JIT/trampoline region.
appendLit(outBuf, sizeof(outBuf), &bp,
" [Mono JIT/trampoline (anon rwxp)]");
} else {
appendLit(outBuf, sizeof(outBuf), &bp, " [anon]");
}
outBuf[bp] = '\0';

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outBuf[...] = ']' write is hard to reason about and can drop the closing bracket when the buffer is full (it writes ] at bp but then outBuf[bp] = '\0' overwrites it when bp didn't increment). Consider simplifying this to an explicit bounds check that always preserves either the bracket+NUL or just a NUL, to keep formatting predictable.

Copilot uses AI. Check for mistakes.
Comment thread osu.Android/mono.env
Comment on lines +17 to +24
# TMPDIR=/storage/emulated/0/Android/data/sh.ppy.osulazer/files
# Redirects Mono's perfmap output into the same external-files directory
# that we already write native_crash.log to. This is the *only* path that
# is (a) writable by the app, (b) readable post-mortem by the user via
# the Files app on an unrooted device, and (c) survives app uninstall on
# most devices' "external files" semantics.
MONO_ENV_OPTIONS=--jitmap
TMPDIR=/storage/emulated/0/Android/data/sh.ppy.osulazer/files

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TMPDIR is hardcoded to /storage/emulated/0/Android/data/sh.ppy.osulazer/files. This will break perfmap output if the applicationId/package name changes (debug variants, forks, or future renames), and may be incorrect on devices where the external storage path differs. Consider generating this value from the actual package name at build time (MSBuild property) or avoiding the hardcoded TMPDIR by having the native side search dirname(g_logPath) only (since it already points at the external-files dir).

Suggested change
# TMPDIR=/storage/emulated/0/Android/data/sh.ppy.osulazer/files
# Redirects Mono's perfmap output into the same external-files directory
# that we already write native_crash.log to. This is the *only* path that
# is (a) writable by the app, (b) readable post-mortem by the user via
# the Files app on an unrooted device, and (c) survives app uninstall on
# most devices' "external files" semantics.
MONO_ENV_OPTIONS=--jitmap
TMPDIR=/storage/emulated/0/Android/data/sh.ppy.osulazer/files
# TMPDIR is intentionally not set here. Hardcoding the external-files path
# would bake in a specific package name and storage root, which breaks for
# debug variants, forks, future renames, or devices where external storage
# is mounted elsewhere. Perfmap discovery should instead rely on the native
# crash handler resolving it relative to dirname(g_logPath).
MONO_ENV_OPTIONS=--jitmap

Copilot uses AI. Check for mistakes.
Comment on lines +257 to +262
// Candidate directories, in priority order. The dir containing g_logPath
// is checked first so a build that sets `TMPDIR=<external-files-dir>`
// (the recommended config) finds its perfmap immediately.
const char* tmpEnv = getenv("TMPDIR");
char logDir[kMaxLogPathLen] = {};
if (g_logPath[0] != '\0') {

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensurePerfmapLoaded() calls getenv("TMPDIR") inside the crash signal handler path. getenv() is not async-signal-safe (can take libc locks / touch malloc state), which risks deadlock or re-entrancy crashes while handling SIGSEGV. Consider removing the getenv() lookup from the handler (e.g. precompute/copy TMPDIR at install time, or only search dirname(g_logPath), /tmp, /data/local/tmp). Also update the async-signal-safety comment block to match the actual syscalls used (mmap/fstat/etc.).

Copilot uses AI. Check for mistakes.
size_t nameStart = p;
while (p < n && d[p] != '\n') ++p;
size_t nameLen = p - nameStart;
if (size != 0 && pc >= start && pc < start + size) {

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In resolveViaPerfmap(), the range check pc < start + size can overflow uint64_t if start is large, which would incorrectly treat unrelated PCs as within-range. Safer pattern is to compare as pc >= start && (pc - start) < size (after ensuring pc >= start).

Suggested change
if (size != 0 && pc >= start && pc < start + size) {
if (size != 0 && pc >= start && (static_cast<uint64_t>(pc) - start) < size) {

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants